Skip to content

Conversation

burrbull
Copy link
Member

@burrbull burrbull commented Aug 4, 2019

r? @therealprof

Add more docs as @Disasm proposed.

Restore unsafe on W::bits() I somewhere lost.

Don't generate write if reset_value is not present. #258

@burrbull burrbull requested a review from a team as a code owner August 4, 2019 06:07
@rust-highfive rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools labels Aug 4, 2019
src/lib.rs Outdated
//!
//! `ResetValue` trait provides `reset_value` which returns the value of the `CR2`
//! register after a reset. This value can be used to modify the
//! writable bitfields of the `CR2` register or reset it in initial state.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//! writable bitfields of the `CR2` register or reset it in initial state.
//! writable bitfields of the `CR2` register or reset it to its initial state.

src/lib.rs Outdated
@@ -256,26 +262,30 @@
//! }
//! ```
//!
//! ## `reset`
//!
//! `ResetValue` trait provides `reset_value` which returns the value of the `CR2`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//! `ResetValue` trait provides `reset_value` which returns the value of the `CR2`
//! The `ResetValue` trait provides `reset_value` which returns the value of the `CR2`

@@ -165,16 +208,18 @@ impl<FI> R<bool, FI> {
}

///Register writer
///
///It is used as argument of closures in [`write`](Reg::write) and [`modify`](Reg::modify) methods of register
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
///It is used as argument of closures in [`write`](Reg::write) and [`modify`](Reg::modify) methods of register
///Used as an argument to the closures in the [`write`](Reg::write) and [`modify`](Reg::modify) methods of the register

@@ -112,6 +151,9 @@ where
}

///Register/field reader
///
///Result of call [`read`](Reg::read) method of register.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
///Result of call [`read`](Reg::read) method of register.
///Result of the [`read`](Reg::read) method of a register.

@@ -112,6 +151,9 @@ where
}

///Register/field reader
///
///Result of call [`read`](Reg::read) method of register.
///Also it can be used in [`modify`](Reg::read) method
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
///Also it can be used in [`modify`](Reg::read) method
///Also it can be used in the [`modify`](Reg::read) method

@@ -100,7 +125,21 @@ where
{
///Modifies the contents of the register
///
///See [modifying](https://rust-embedded.github.io/book/start/registers.html#modifying) in book.
///Change only part of register:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
///Change only part of register:
///E.g. to do a read-modify-write sequence to change parts of a register:

///or
///```ignore
///periph.reg.modify(|_, w| w
/// .field1() .bits(newfield1bits)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// .field1() .bits(newfield1bits)
/// .field1().bits(newfield1bits)

///```ignore
///periph.reg.modify(|_, w| w
/// .field1() .bits(newfield1bits)
/// .field2() .set_bit()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// .field2() .set_bit()
/// .field2().set_bit()

///periph.reg.modify(|_, w| w
/// .field1() .bits(newfield1bits)
/// .field2() .set_bit()
/// .field3() .variant(VARIANT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// .field3() .variant(VARIANT)
/// .field3().variant(VARIANT)

///or write only fields you need:
///```ignore
///periph.reg.write(|w| w
/// .field1() .bits(newfield1bits)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// .field1() .bits(newfield1bits)
/// .field1().bits(newfield1bits)

///```ignore
///periph.reg.write(|w| w
/// .field1() .bits(newfield1bits)
/// .field2() .set_bit()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// .field2() .set_bit()
/// .field2().set_bit()

///periph.reg.write(|w| w
/// .field1() .bits(newfield1bits)
/// .field2() .set_bit()
/// .field3() .variant(VARIANT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// .field3() .variant(VARIANT)
/// .field3().variant(VARIANT)

@@ -68,7 +79,19 @@ where
{
///Writes bits to `Writable` register
///
///See [writing](https://rust-embedded.github.io/book/start/registers.html#writing) in book.
///You can write raw bits into register:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
///You can write raw bits into register:
///You can write raw bits into a register:

///```ignore
///periph.reg.write(|w| unsafe { w.bits(rawbits) });
///```
///or write only fields you need:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
///or write only fields you need:
///or write only the fields you need:

@@ -42,7 +42,16 @@ where
{
///Reads the contents of `Readable` register
///
///See [reading](https://rust-embedded.github.io/book/start/registers.html#reading) in book.
///You can read contents of register in such way:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
///You can read contents of register in such way:
///You can read the contents of a register in such way:

///```ignore
///let bits = periph.reg.read().bits();
///```
///or get contents of particular field of register.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
///or get contents of particular field of register.
///or get the content of a particular field of a register.

@@ -55,6 +64,8 @@ where
U: Copy,
{
///Writes the reset value to `Writable` register
///
///After call this method register takes the initial state
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
///After call this method register takes the initial state
///Resets the register to its initial state

Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also should have a CHANGELOG entry mentioning the do improvements and examples. And I think the unsafe problem has snuck in in V0.15.0 because I was looking at weird "don't need unsafe here" and after removal "you do need unsafe here" warning/errors just yesterday; in that case we also need a CHANGELOG entry for this.

@burrbull
Copy link
Member Author

burrbull commented Aug 4, 2019

Done. Rebased.

Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@therealprof
Copy link
Contributor

bors r+

bors bot added a commit that referenced this pull request Aug 4, 2019
365: fix docs+examples, work without reset, fixes r=therealprof a=burrbull

r? @therealprof 

Add more docs as @Disasm proposed.

Restore `unsafe` on `W::bits()` I somewhere lost.

Don't generate `write` if `reset_value` is not present. #258 

Co-authored-by: Andrey Zgarbul <[email protected]>
@bors
Copy link
Contributor

bors bot commented Aug 4, 2019

Build succeeded

@bors bors bot merged commit fe27531 into rust-embedded:master Aug 4, 2019
@burrbull burrbull deleted the docs branch August 4, 2019 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants